Skip to content

Conversation

@jgrund
Copy link

@jgrund jgrund commented Feb 9, 2026

Without this, the QUIC CONNECTION_CLOSE frame sent when the caller closes the connection can race with (and arrive before) the stream data at the server, causing the server's accept_bi to see ApplicationClosed instead of the notify message.

Call finish() to signal end-of-stream, then stopped().await to wait for the peer's transport-level ACK before returning. This ensures the server has the stream data before the connection is closed.

Without this, the QUIC CONNECTION_CLOSE frame sent when the caller
closes the connection can race with (and arrive before) the stream
data at the server, causing the server's accept_bi to see
ApplicationClosed instead of the notify message.

Call finish() to signal end-of-stream, then stopped().await to wait
for the peer's transport-level ACK before returning. This ensures the
server has the stream data before the connection is closed.

Signed-off-by: Joe Grund <grundjoseph@gmail.com>
Copy link
Member

@matheus23 matheus23 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you so much for looking into this!

Unfortunately, waiting for .stopped().await is not the correct way to ensure all data is sent (as we had to learn the hard way).

But I agree with the spirit of the PR of making sure the connections never get dropped before a request is finished.

One way of making that possible is to use the second stream in a request for signaling, even if the stream itself doesn't have any data sent:

let (mut tx, mut rx) = request.write(msg).await?;
tx.finish()
    .map_err(|e| rpc::WriteError::from(quinn::WriteError::from(e)))?;
rx.received_reset()
    .await
    .map_err(|e| rpc::WriteError::from(quinn::WriteError::from(e)))?;

Then the other side of that protocol that answers requests needs to make sure to call tx.finish() when it fully read and processed the request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants